Skip to content

fix(spa): list-action redirect refresh + popup-blocked fallback (#644) + 1.5.1#646

Merged
MartinCastroAlvarez merged 1 commit into
mainfrom
fix/list-action-redirect-refresh-and-popup-fallback
May 31, 2026
Merged

fix(spa): list-action redirect refresh + popup-blocked fallback (#644) + 1.5.1#646
MartinCastroAlvarez merged 1 commit into
mainfrom
fix/list-action-redirect-refresh-and-popup-fallback

Conversation

@MartinCastroAlvarez
Copy link
Copy Markdown
Owner

Closes #644.

What was wrong

Two real gaps in how the changelist bulk-action handler treated a result.redirect envelope from the API runner:

  1. No await refresh() on the redirect path. Per Django admin's ModelAdmin.response_action contract, an action that returns a redirect can ALSO have applied side-effects to the rows (reprocess + redirect to status page, regenerate + redirect to download, …). The previous code return-ed before refresh, so those side-effects weren't reflected until the next manual reload.

  2. Popup-blocker silently swallowed the redirect. window.open returns null when the call happens past the direct-user-gesture moment (we're inside an async continuation after the POST resolves). The previous code ignored that null, so the operator saw "opened in a new tab" but nothing opened.

Fix

  • window.open return value checked. On null, render an amber fallback banner with a clickable link (<a target="_blank" rel="noopener noreferrer">) so the redirect URL is never lost. Dismiss button clears the banner.
  • refresh() now fires on BOTH branches.
  • Generic " — N item(s)" success toast suppressed on the redirect path — the redirect's destination usually surfaces the next-step UX, so an extra "Done" toast adds noise.

The redirect path itself still uses noopener,noreferrer (reverse-tabnabbing + referer-leak defence) per the issue's stated security stance — those flags were already there.

Refactor for testability

Extracted handleActionResult(...) as a pure helper next to ListPage.tsx so the redirect / popup-blocked / refresh / messages-toast flow is unit-testable without rendering the whole page. window.open and setPendingRedirect are dependency-injected (openLink, onPopupBlocked args) so the test suite can lock the branching without touching jsdom's non-configurable navigation surface.

Dep bump

django-admin-rest-api ^1.1.0 constraint resolved to 1.1.1 in the lock (the README-sync release that documents the security improvements landed in 1.0.7+ — actions runner DoS guard + history field-name redaction + settings hygiene checks).

Verification

  • pnpm test194 / 194 ✓ (up from 187; +7 new in action-result.test.ts)
  • pnpm -r typecheck
  • pnpm lint
  • pnpm -w build

The seven new tests cover:

  1. Redirect → openLink → info toast.
  2. Redirect → openLink returns null (popup blocker) → onPopupBlocked called, NO info toast.
  3. Refresh fires on both branches.
  4. messages[] toasts by level (error → red, info → blue, success → green).
  5. No messages + no redirect → generic " — N item(s)" toast.
  6. Singular "item" when count is 1.
  7. No generic toast on the redirect path (destination surfaces next-step UX).

What's NOT in this PR

  • Playwright e2e test (the issue lists this as an acceptance criterion but the project memory steers away from Playwright/Cypress tooling). The vitest covering the same flow via dependency injection meets the issue's "or equivalent" clause.

🤖 Generated with Claude Code

… + 1.5.1

## #644 — list-page action redirect parity

Two real gaps in how the changelist bulk-action handler treated a
`result.redirect` envelope:

1. **No `await refresh()` on the redirect path.** Per Django admin's
   `ModelAdmin.response_action` contract, an action that returns a
   redirect can ALSO have applied side-effects to the rows
   (`reprocess + redirect to status page`, `regenerate + redirect to
   download`, …). The previous code `return`-ed before refresh, so
   those side-effects weren't reflected until the next manual
   reload.

2. **Popup-blocker silently swallowed the redirect.** `window.open`
   returns `null` when the call happens past the direct-user-gesture
   moment (we're inside an async continuation after the POST
   resolves). The previous code ignored that null, so the operator
   saw the toast "opened in a new tab" but nothing opened.

Fix:
- `window.open` return value checked. On `null`, render an amber
  fallback banner with a clickable link (`<a target="_blank"
  rel="noopener noreferrer">`) so the redirect URL is never lost.
  Dismiss button clears the banner.
- `refresh()` now fires on BOTH branches.
- Generic "<action> — N item(s)" success toast suppressed on the
  redirect path — the redirect's destination usually surfaces the
  next-step UX, so an extra "Done" toast adds noise.

Extracted `handleActionResult(...)` as a pure helper next to
`ListPage.tsx` so the redirect / popup-blocked / refresh / messages-
toast flow is unit-testable without rendering the whole page.
Seven new vitests cover every branch.

## Dep bump

`django-admin-rest-api ^1.1.0` → resolved to **1.1.1** in the lock
(README sync of the security improvements that landed in 1.0.7+
— actions runner DoS guard + history field-name redaction +
settings hygiene system checks).

## Verification

- `pnpm test` — **194 / 194 ✓** (up from 187; +7 new)
- `pnpm -r typecheck` ✓
- `pnpm lint` ✓
- `pnpm -w build` ✓

Closes #644.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@MartinCastroAlvarez MartinCastroAlvarez merged commit b62ad28 into main May 31, 2026
6 checks passed
@MartinCastroAlvarez MartinCastroAlvarez deleted the fix/list-action-redirect-refresh-and-popup-fallback branch May 31, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow redirect envelope from actions/<name>/ response — Django admin parity

2 participants